fix(s09): re-locate the memory turn by identity after compaction instead of a stale index - #497
Closed
ClearVIper3 wants to merge 1 commit into
Closed
Conversation
`memory_turn` was computed once as `len(messages) - 1` *before* the s08 compression pipeline runs. `snip_compact` / `compact_history` / `reactive_compact` shorten or rebuild the message list, so the index drifts: the guard `memory_turn < len(messages)` then fails and the memory injection is silently skipped — precisely when compaction triggers (long conversations), i.e. when the memory system is supposed to matter most. Fix: capture the user-turn *object* and re-locate it by identity after each compaction step via the new `_find_memory_turn`, falling back to the last str-content user turn if the query was summarized away. No change to the s08 compaction functions — the bug was entirely in agent_loop.
Collaborator
|
Thanks for identifying and fixing the stale memory_turn index in the previous s09 implementation. The issue was valid for that version. Since then, s09 has been refactored to load relevant memories directly into the system prompt and no longer uses a pre-compaction message index. The same index-based path has also been removed from the integrated implementation. Since the original issue no longer exists on main and this PR now conflicts with the current architecture, I’m closing it as superseded. Thanks again for the contribution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The memory injection turn was located by a pre-compaction index that
compaction then shifted, so memory was silently dropped whenever
compaction fired. Re-locate the user turn by object identity after each
compaction step instead of trusting the stale index; leave the
compaction functions untouched.
Fixes #496